Platform-specific vsix #89

paulacamargo25 · 2023-09-21T16:35:34Z

Closed: microsoft/vscode-python#20748

isidorn · 2023-09-22T09:07:54Z

build/azure-pipeline.pre-release.yml

@@ -28,6 +28,34 @@ extends:
  template: azure-pipelines/extension/pre-release.yml@templates
  parameters:
    l10nSourcePaths: ./src
+    buildPlatforms:
+      - name: Linux
+        vsceTarget: ''


Is this line needed? There is no vsceTarget?

This will create a platform independent VSIX. @paulacamargo25, is that the intention here? Thanks!

Yes, according to what I understood, I always need one that does not depend on any platform, correct?

@isidorn, @sandy081 to confirm that.

Yes, according to what I understood, I always need one that does not depend on any platform, correct?

What does this platform independent VSIX contain? It is not necessary if you are building VSIXs for all platforms.

Thanks for the review, it contains the debugpy wheel which is not specific to any platform.

If you are building VSIX for all platforms then you do not need a general VSIX.

isidorn

Overall the PR looks good to me. I am also assigning to @lszomoru so he gives it a glance since he owns the extension publish pipelines.

brettcannon

There's a critical miss of exiting on failure of hash validation.

noxfile.py

brettcannon

Some clean-up in the download function to use some of the earlier changes you made.

noxfile.py

karthiknadig · 2023-09-25T22:54:48Z

noxfile.py

-
-def _update_pip_packages(session: nox.Session) -> None:
-    session.run("pip-compile", "--generate-hashes", "--upgrade", "./requirements.in")
+DEBUGPY_WHEEL_URLS = {


Any reason to check in these values rather than pull from here: https://pypi.org/pypi/debugpy/json

You could set it up to take the latest with 1.7.*. The same code is then reusable for the 2.7 and 3.6 cases.

Not really, when I talked to Brett we decided to have the dict since debugpy didn't release new versions often.

Ok. I would prefer to have the same noxfile across the 3 variants... and only have the version change between them

It depends on the security posture we want to take. If we are just doing this to make sure the download isn't corrupted or we didn't download the wrong file, then it's fine (although it does require an extra HTTP request). But if we are trying to avoid any potential PyPI compromise then static hash values mitigate that risk.

@karthiknadig what "3 variants" are you referring to?

And we can always store the information in a .json file if the key concern is just the code not changing instead of recording the hashes offline.

brettcannon

Missing some key pip install flags to avoid any supply-chain attacks.

build/azure-pipeline.pre-release.yml

debugpy_info.json

noxfile.py

brettcannon · 2023-10-18T20:59:15Z

noxfile.py

+    with url_lib.urlopen(value["url"]) as response:
+        data = response.read()
+        if (
+            hashlib.new(hash_algorithm, data).hexdigest()


I don't see how you're using the hash algorithm as specified in the JSON data, and instead are just assuming it's always sha256. The reason to write it in the JSON that the hash is for sha256 is so you can get the hash algorithm from the JSON data and not have to hard-code that in the Python code.

brettcannon · 2023-10-18T21:10:18Z

noxfile.py

-    session.log(f"Updating version from {package_json['version']} to {version}")
-    package_json["version"] = version
-    package_json_path.write_text(json.dumps(package_json, indent=4), encoding="utf-8")
+def create_debugpy_json(session: nox.Session, version="1.7.0", cp="cp311"):


I don't see where this is called (maybe it's meant to be manual), but is there a reason to hard-code the Debugpy and Python version? It seems like something that should be stored in the JSON file so that this file requires no changes when we upgrade to a new Debugpy version or Python version.

Not really, i just wanted to have a default value just in case as , but i can remove this values. For other versions it would only be necessary to run the session with the parameters that we need through the terminal, this file will not need to be changed.

* Update nox file and pre release * update args * update value * see params * echo vars * fix args * fix args * Fix error obj to string * fix command line error * update args * update how access values * Add args * Send arguments * tried ith arguments * update value * remove args extra * update run * send as env * add to see other values * update env variables * use VSCETARGET * fix variable * Fix macOs error * Fix lint in nox file * Fix nox hash comparison * Fix default value * Remove unnecessary code * Update platform url and debugpy version * Fix dict key name * Remove vsix for all platforms * Fix pr comments * fix download function * fix hash code * Read from json pypypackage * add json file and session that creates it * Reformat nox file * resolve comments * fix lint error * update pipeline * fix build error * dont hardcode the hash

* update pre release (#71) * Add port provider (#85) * Add port attribute provider * Add message * Enabled proposed api * Ignore all the ports * Fix format * fix pr comments * Add regex for terminal command * Add adapter to pattern * Add global configuration for justMyCode (#91) * Aff configuration * Read just my code from settings too * fix tests * fix lint * Update extension name (#84) * Update package.json * Fix readme * update debug type name * Fix tests * Fix lint * Fix test * Update translations * revert extension id * Fix tests * fix lint * Update debug type name * Fix translations and lint * update debug type * fix lint * use version 1.7 (#95) * Remove default justMyCode (#100) * Fix error in workspace (#101) * Fix error in workspace * fix test and lint * Fix format * Bump mheap/github-action-required-labels from 3 to 5 (#23) Bumps [mheap/github-action-required-labels](https://github.com/mheap/github-action-required-labels) from 3 to 5. - [Release notes](https://github.com/mheap/github-action-required-labels/releases) - [Commits](mheap/github-action-required-labels@v3...v5) --- updated-dependencies: - dependency-name: mheap/github-action-required-labels dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @typescript-eslint/parser from 5.59.11 to 5.62.0 (#61) Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 5.59.11 to 5.62.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.62.0/packages/parser) --- updated-dependencies: - dependency-name: "@typescript-eslint/parser" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump typescript from 5.1.3 to 5.2.2 (#79) Bumps [typescript](https://github.com/Microsoft/TypeScript) from 5.1.3 to 5.2.2. - [Release notes](https://github.com/Microsoft/TypeScript/releases) - [Commits](microsoft/TypeScript@v5.1.3...v5.2.2) --- updated-dependencies: - dependency-name: typescript dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update npm packages (#105) * update package.json * Fux lint * Update to node 18 (#106) * Platform-specific vsix (#89) * Update nox file and pre release * update args * update value * see params * echo vars * fix args * fix args * Fix error obj to string * fix command line error * update args * update how access values * Add args * Send arguments * tried ith arguments * update value * remove args extra * update run * send as env * add to see other values * update env variables * use VSCETARGET * fix variable * Fix macOs error * Fix lint in nox file * Fix nox hash comparison * Fix default value * Remove unnecessary code * Update platform url and debugpy version * Fix dict key name * Remove vsix for all platforms * Fix pr comments * fix download function * fix hash code * Read from json pypypackage * add json file and session that creates it * Reformat nox file * resolve comments * fix lint error * update pipeline * fix build error * dont hardcode the hash * Fix GDPR comments (#113) * Update readme (#111) * Update readme * Update README.md Co-authored-by: Luciana Abud <45497113+luabud@users.noreply.github.com> --------- Co-authored-by: Luciana Abud <45497113+luabud@users.noreply.github.com> * Remove build folder from bundled vsix (#120) * Update debugpy version * Update nox and pipelines * update json file * fix actions * fix 3.6 error * fix error running with 3.6 * fix lint * update python version * fix error in lint pipeline * update pipelines * fix nox file * fix tests * fix pipeline error * fix lint * use python 3.6 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Logan Ramos <lramos15@gmail.com> Co-authored-by: Luciana Abud <45497113+luabud@users.noreply.github.com> Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>

paulacamargo25 added this to the September 2023 milestone Sep 21, 2023

paulacamargo25 added the debt Covers everything internal: CI, testing, refactoring of the codebase, etc. label Sep 21, 2023

paulacamargo25 self-assigned this Sep 21, 2023

paulacamargo25 force-pushed the platform-specific-vsix branch from 6896422 to db4610a Compare September 21, 2023 17:12

paulacamargo25 requested a review from isidorn September 21, 2023 20:23

paulacamargo25 marked this pull request as ready for review September 21, 2023 20:23

isidorn reviewed Sep 22, 2023

View reviewed changes

isidorn requested a review from lszomoru September 22, 2023 09:09

isidorn previously approved these changes Sep 22, 2023

View reviewed changes

paulacamargo25 dismissed isidorn’s stale review via 58ed48f September 25, 2023 19:21

sandy081 previously approved these changes Sep 25, 2023

View reviewed changes

brettcannon suggested changes Sep 25, 2023

View reviewed changes

paulacamargo25 dismissed sandy081’s stale review via d0a5784 September 25, 2023 22:28

paulacamargo25 force-pushed the platform-specific-vsix branch from d0a5784 to 18908fa Compare September 25, 2023 22:30

paulacamargo25 requested a review from brettcannon September 25, 2023 22:31

brettcannon suggested changes Sep 25, 2023

View reviewed changes

noxfile.py Outdated Show resolved Hide resolved

noxfile.py Outdated Show resolved Hide resolved

noxfile.py Outdated Show resolved Hide resolved

brettcannon requested a review from karthiknadig September 25, 2023 22:49

karthiknadig reviewed Sep 25, 2023

View reviewed changes

paulacamargo25 requested a review from brettcannon September 25, 2023 23:30

paulacamargo25 modified the milestones: September 2023, October 2023 Sep 26, 2023

paulacamargo25 force-pushed the platform-specific-vsix branch 2 times, most recently from 5a42ab8 to f03b5de Compare October 11, 2023 20:47

paulacamargo25 added 7 commits October 11, 2023 13:50

Update nox file and pre release

52db410

update args

6c74285

update value

f3a2e09

see params

9aa1789

echo vars

c97c370

fix args

8080291

fix args

5ce94a9

paulacamargo25 added 8 commits October 11, 2023 13:50

Fix dict key name

b233ac5

Remove vsix for all platforms

1669ea0

Fix pr comments

c9308ab

fix download function

2b43b10

fix hash code

4f7e706

Read from json pypypackage

cf2ed03

add json file and session that creates it

5c95bd3

Reformat nox file

f64c727

paulacamargo25 force-pushed the platform-specific-vsix branch from f03b5de to f64c727 Compare October 11, 2023 20:50

paulacamargo25 requested a review from karthiknadig October 11, 2023 20:54

karthiknadig previously approved these changes Oct 13, 2023

View reviewed changes

brettcannon suggested changes Oct 16, 2023

View reviewed changes

resolve comments

1bac5b0

paulacamargo25 dismissed karthiknadig’s stale review via 1bac5b0 October 17, 2023 07:42

paulacamargo25 added 3 commits October 17, 2023 00:48

fix lint error

ebf13d8

update pipeline

95a257b

fix build error

b436f0b

paulacamargo25 requested a review from brettcannon October 17, 2023 19:59

paulacamargo25 enabled auto-merge (squash) October 18, 2023 17:18

paulacamargo25 removed the request for review from lszomoru October 18, 2023 20:14

brettcannon suggested changes Oct 18, 2023

View reviewed changes

dont hardcode the hash

9070859

paulacamargo25 disabled auto-merge October 18, 2023 21:31

paulacamargo25 requested a review from brettcannon October 18, 2023 21:33

brettcannon approved these changes Oct 18, 2023

View reviewed changes

paulacamargo25 merged commit 25425b5 into main Oct 18, 2023
18 checks passed

paulacamargo25 deleted the platform-specific-vsix branch October 18, 2023 22:01

imphil mentioned this pull request May 2, 2024

The 'Python Debugger' extension is not available in VSCodium for Linux ARM 64 #318

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Platform-specific vsix #89

Platform-specific vsix #89

paulacamargo25 commented Sep 21, 2023 •

edited

Loading

isidorn Sep 22, 2023

lszomoru Sep 22, 2023

paulacamargo25 Sep 22, 2023

lszomoru Sep 22, 2023

sandy081 Sep 25, 2023

paulacamargo25 Sep 25, 2023

sandy081 Sep 25, 2023

isidorn left a comment

brettcannon left a comment

brettcannon left a comment

karthiknadig Sep 25, 2023

karthiknadig Sep 25, 2023

paulacamargo25 Sep 25, 2023

karthiknadig Sep 25, 2023

brettcannon Sep 25, 2023

brettcannon Sep 26, 2023

brettcannon left a comment

brettcannon Oct 18, 2023

brettcannon Oct 18, 2023

paulacamargo25 Oct 18, 2023

Platform-specific vsix #89

Platform-specific vsix #89

Conversation

paulacamargo25 commented Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isidorn left a comment

Choose a reason for hiding this comment

brettcannon left a comment

Choose a reason for hiding this comment

brettcannon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brettcannon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulacamargo25 commented Sep 21, 2023 •

edited

Loading